[DO_NOT_MERGE_YET] rotors_hil_interface: take advantage of MAVROS hil plugin#438
[DO_NOT_MERGE_YET] rotors_hil_interface: take advantage of MAVROS hil plugin#438TSC21 wants to merge 3 commits intoethz-asl:masterfrom
Conversation
|
Can one of the admins verify this patch? |
|
@hitimo @pvechersky @helenol can you please take a look? Thanks |
| // Default values | ||
| static constexpr bool kDefaultSensorLevelHil = true; | ||
| static constexpr double kDefaultHilFrequency = 100.0; | ||
| static constexpr double kDefaultBodyToSensorsRoll = M_PI; |
There was a problem hiding this comment.
My only question is here: on MAVROS, we apply ENU to NED transform (https://github.com/mavlink/mavros/blob/master/mavros/src/lib/ftf_frame_conversions.cpp#L30) and then base_link to body/aircraft frame (https://github.com/mavlink/mavros/blob/master/mavros/src/lib/ftf_frame_conversions.cpp#L37), while you just seem to apply this last one. Which frame of reference are we using in rotors_gazebo?
There was a problem hiding this comment.
I believe Rotors uses the standard ROS reference frame of NWU, hence the rotation by 180 degrees around the x axis to convert it to NED in the HIL interface node in this case.
|
@hitimo @pvechersky @helenol can you give me your thoughts on this? |
pvechersky
left a comment
There was a problem hiding this comment.
Conceptually, the changes are good. I think as much of MAVROS's built-in functionality should be leveraged by our HIL module, and using the hil plugin is a good step in that direction.
I just have a couple of higher-level comments that the other reviewers might follow-up on:
-
In rotors_hil.rosinstall you change the MAVROS uri to the mavlink repo instead of the ethz-asl one. The switch to ethz-asl repo for MAVROS was done after my time of working on this HIL interface so I'm not sure if the Rotors admins would prefer to stick to the ethz-asl repo or not. But if the ethz-asl repo is preferred then perhaps relevant hil plugin changes in MAVROS can be merged into that repo?
-
Code formatting. Rotors used Google coding standards I believe. See other C++ code in the project for reference.
Thank for integrating these improvements!
|
Can one of the admins verify this patch? |
|
ok to test |
|
Hi, I would like to use HIL and I'm blocked by #367 which I understand would be solved by this. What is needed to finish this PR? |
|
Hi, anyone still looking into this? I tried this PR and everything mostly works. I have only two issues:
|
|
@v01d I got too much on my back to finish this, as I am also supporting and maintaining |
|
Cool thanks. |
|
Another question, is the mavlink interface plugin required? I'm not really sure what it is for, since the hil interface seems to do all the talking to mavros. |
|
@TSC21 How should we move forward with this? |
|
test this please |
With this PR we take (almost) full advantage of MAVROS hil plugin. There, the unit conversions are applied, besides the frame transform to NED. I also applied the uncrustify style for C++. Let me know if you want me to revert this step.